r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

A question to @traviscross ...

Should we also need to run this through t-lang as well? It is hard to imagine that anything would rely on destructor called in this way, but I am not sure.

A question to @traviscross ...

Should we also need to run this through t-lang as well? It is hard to imagine that anything would rely on destructor called in this way, but I am not sure.

Well, let's see. It looks like this issue does affect the user-visible stable behavior of the language:

use core::{mem, ptr, sync::atomic::{AtomicU64, Ordering}};

static COUNT: AtomicU64 = AtomicU64::new(0);

struct S;
impl Drop for S {
    fn drop(&mut self) {
        COUNT.fetch_add(1, Ordering::Relaxed);
    }
}
trait Tr {}
impl Tr for S {}

const X: Box<dyn Tr> = unsafe {
    mem::transmute::<_, Box<S>>(ptr::dangling_mut::<S>())
};

fn main() {
    assert_eq!(0, COUNT.load(Ordering::Relaxed));
    let _a: &'static [Box<dyn Tr>; 0] = &[X; 0];
    assert_eq!(1, COUNT.load(Ordering::Relaxed));
    //~^ Drop was run.
    let _b = [X; 0];
    assert_eq!(1, COUNT.load(Ordering::Relaxed));
    //~^ Drop wasn't run.
}

So, yes, it's ours to fix via FCP.

Anyone think we need a crater run? For my part, I could go either way on that, and I do want us to fix this, so let's:

@rfcbot fcp merge

@rustbot labels +I-lang-nominated +needs-fcp

cc @rust-lang/lang @rust-lang/lang-advisors

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

Another fun edge case where this change is visible in user code: This code compiles with this PR, but fails on nightly due to dropping in const

use std::rc::Weak;

const X: Weak<i32> = Weak::new();
const Y: [Weak<dyn Send>; 0] = [X; 0];

The code changes seem fine, speaking as a reviewer who is not at all familiar with the MIR building and relevant lang details. The example from @theemathas might be worth adding to the test case.

@bors try

Okay, I will request a crater run ahead of the next meeting.

I will push a commit to include the second test cases.

☀️ Try build successful (CI)
Build commit: 981ca44 (981ca440e18f6cc4bc9026d4be8ddb1ca8378af7, parent: a1531335fe2807715fff569904d99602022643a7)

Hello team. I would like to request

@ craterbot run mode=build-and-test crates=top-100

for a quick triage. 🙇

Let's do the top 1000, just for more coverage :)

@craterbot run mode=build-and-test crates=top-1000

👌 Experiment pr-145277 created and queued.
🤖 Automatically detected try build 981ca44
⚠️ Try build based on commit c1ab3ed, but latest commit is 8444dde. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

⚠️ Try build based on commit c1ab3ed, but latest commit is 8444dde. Did you forget to make a new try build?

The try build was done before the second commit was added, but it's just a test, so this shouldn't matter.

🚧 Experiment pr-145277 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-145277 is completed!
📊 1 regressed and 2 fixed (1000 total)
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

1 regression from dbus-0.9.7 because libc-v0.2.139 was not available on crates.io.

I.e., all the regressions were spurious.

Does this relate to #79580 somehow?

r=me once the lang team has approved.

@RalfJung Yes. I'm not sure if I'm misunderstanding that issue (which seems to have an outdated description). But this PR is fixing the behavior to be correct, right?

I'm not sure, I didn't page this back in -- I just remembered seeing something about empty arrays before.